xy data documentation #6736
xy data documentation #6736
Conversation
|
Sorry for the late reply. @ayushjariyal could you resolve the conflict and fix the pre-commit error. We'll review it then. |
|
@unkcpz I accidentally deleted my previous branch and can no longer undo my previous changes, so I have opened a new PR. Could you please review it? |
|
Sorry, I think you didn't delete it. It still here https://github.com/ayushjariyal/aiida-core/tree/issue_XyData_documentation |
|
Oh, I see! Thanks for letting me know. I’ll check on it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6736 +/- ##
==========================================
+ Coverage 78.11% 78.12% +0.02%
==========================================
Files 564 564
Lines 42542 42543 +1
==========================================
+ Hits 33228 33234 +6
+ Misses 9314 9309 -5 ☔ View full report in Codecov by Sentry. |
ayushjariyal
force-pushed
the
issue_XyData_documentation
branch
2 times, most recently
from
329bc35 to
a9e7df5
Compare
ayushjariyal
force-pushed
the
issue_XyData_documentation
branch
from
cffc576 to
ce6f8ea
Compare
ayushjariyal
force-pushed
the
issue_XyData_documentation
branch
from
e3888a7 to
71a6017
Compare
ayushjariyal
force-pushed
the
issue_XyData_documentation
branch
from
0be89c2 to
fb66e7d
Compare
|
@unkcpz I am not able to understand why these checks have failed. Can you please help me figure that out? |
Don't worry, it is auto-fixed by pre-commit action, CI is passed. |
There was a problem hiding this comment.
Thanks @ayushjariyal. Some requests:
- The documentation should also changed, it is in
aiida-core/docs/source/topics/data_types.rst
Line 270 in 290045b
- Could you add a description in the PR to describe what you have changed and why?
- Can you add a test to test the new behavior?
Since the behavior is changed, it should be regard as a break change (as a bugfix). @agoscinski Therefore, I will add it to with tag v2.8.0.
ayushjariyal
force-pushed
the
issue_XyData_documentation
branch
from
43fd428 to
37abafa
Compare
|
Thanks for update the PR @ayushjariyal. I am aware of your change, but please don't keep on click the merge with main button, I got notification everytime ;) I'll review it soon, I am quite packed until Thursday. |
|
Thank you for the update! Apologies for the repeated notifications—I didn’t realize it was causing any inconvenience. I’ll make sure to avoid clicking the merge button going forward. Looking forward to your review whenever you get the time. |
fix #6731
Refactored
get_y()and Introducedget_y_arraynames():Split the functionality of
get_y()into two distinct methods:get_y(): Returns the Y arrays along with their names and units.get_y_arraynames(): Returns only the names of the Y arrays. This improves API clarity and usability.Update Documentation:
XyDatafor easier understanding.index.rstdocumentation to reflect the new API changes and ensure consistency.Added Test for
get_y_arraynames():get_y_arraynames().